Skip to content

fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema (restore --colors/--highlight)#1030

Merged
zhengzhijiej-tech merged 11 commits into
larksuite:feat/lark-sheets-refactorfrom
zhengzhijiej-tech:fix/sheets-e2e-fixes-batch1
May 25, 2026
Merged

fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema (restore --colors/--highlight)#1030
zhengzhijiej-tech merged 11 commits into
larksuite:feat/lark-sheets-refactorfrom
zhengzhijiej-tech:fix/sheets-e2e-fixes-batch1

Conversation

@zhengzhijiej-tech
Copy link
Copy Markdown
Collaborator

@zhengzhijiej-tech zhengzhijiej-tech commented May 22, 2026

Summary

Six independent schema-alignment fixes for E2E test report cases on this branch
(commit 1e05e7b, PPE lane ppe_lark_cli_sheet). Each matches a specific
server error that traces back to a CLI ↔ canonical-schema mismatch.

# Shortcut Server error Root cause
1 +workbook-create --headers/--values set_cell_range: sheet_id or sheet_name is required buildInitialFillInput sent {"sheet_id":""} (assumed server-side default fallback that doesn't exist)
2 +dropdown-set / -update data_validation.{colors,multiple_values,highlight_options} unexpected property CLI emitted field names that didn't exist in canonical set_cell_range.data_validation
3 +dropdown-get sheet not found, sheetId: (empty) dropdownGetInput sent Sheet1!C2:C6 verbatim instead of splitting into sheet_name + bare A1
4 +dim-move [9499] Missing required parameter: Dimension Wrong v2 endpoint (/dimension_range = insert) + camelCase body key
5 +range-sort required property "column"/"ascending" missing --sort-keys items passed through unvalidated; field-naming drift
6 +dropdown-set / -update (follow-up) --colors / --highlight revived after canonical schema gained highlight_colors / enable_highlight; further loosened to allow shorter color arrays after the schema description was relaxed upstream

Fixes per item

#1 workbook-create initial fill — replace {"sheet_id":""} with
{"sheet_name":"Sheet1"}. New workbooks always start with that default sheet;
using sheet_name avoids an extra get_workbook_structure round-trip just to
learn the auto-generated id.
lark_sheet_workbook.go:694-699

#2 dropdown data_validation field renames — align body fields with
canonical schema in sheet-skill-spec/canonical-spec/tool-schemas/tools-schema.json:

  • valuesitems (option list)
  • multiple_valuessupport_multiple_values

(The colors / highlight_options deletion that was originally part of this
item has been undone by #6 below — keep reading.)

Skill ref docs (lark-sheets-write-cells.md, lark-sheets-batch-update.md)
synced.
lark_sheet_write_cells.go

#3 dropdown-get range prefix — reuse the existing splitSheetPrefixedRange
helper to break Sheet1!C2:C6 into sheet_name + bare A1, then thread through
sheetSelectorForToolInput (same pattern as +cells-get).
lark_sheet_read_data.go:249-262

#4 dim-move endpoint + body key — switch URL from /dimension_range
(v2 insert) to /move_dimension; rename body key destinationIndex
destination_index for v2 contract consistency.
lark_sheet_sheet_structure.go:573,598,621

#5 range-sort item validation — walk --sort-keys client-side, reject items
missing column (string) or ascending (bool) with per-item error messages.
Test fixtures and the batch-op contract fixture switched from the historical
{col, order} vocabulary (which the server has never accepted) to the correct
{column, ascending}.
lark_sheet_range_operations.go:590-625

#6 dropdown --colors / --highlight restored, rewired to canonical fields, then length rule loosened

Two-step partial-revert of #2 (commits 8672d8e then 538eb2e):

When #2 landed, canonical set_cell_range.data_validation only knew
help_text / items / operator / range / support_multiple_values / type / values, so --colors / --highlight had no server-side target and were
removed. Two upstream syncs later changed that:

  • 2026-05-22 sync: gained enable_highlight (bool) + highlight_colors
    (string[]). Flags brought back, rewired to the new field names.
  • 2026-05-25 sync: highlight_colors description loosened — length may
    now be shorter than items (server cycles remaining slots through a
    built-in 10-color palette). Previously the description required strict
    equality.

New mapping:

CLI flag data_validation field Was (pre-#2)
--colors (string array, hex) highlight_colors colors
--highlight (bool) enable_highlight highlight_options

Go-side details across 8672d8e + 538eb2e:

  • buildDropdownValidation: re-emits both new fields; --colors length
    check is "must not exceed --options length" (loosened from strict
    equality to match the new schema).
  • validateDropdownOptionsvalidateDropdownOptionsColors: restored
    Validate-time check for +dropdown-update / +dropdown-delete.
  • Tests:
    • TestDropdownSet_CellsShape extended to assert highlight_colors /
      enable_highlight (and assert legacy colors / highlight_options
      absent).
    • TestDropdownSet_ColorsLongerThanOptions (renamed from
      _ColorsLengthMismatch): covers the overflow path with 3 colors vs
      2 options.
    • TestDropdownSet_ColorsShorterAccepted (new): 2 colors vs 4 options
      is legal and forwarded as-is.
    • TestDropdownUpdate_BatchPayload: extended to cover propagation
      through dropdownBatchInput.
  • Synced from spec: skills/lark-sheets/references/lark-sheets-*.md,
    shortcuts/sheets/data/{flag-defs,flag-schemas}.json.

The --options → items and --multiple → support_multiple_values renames
from #2 are preserved — only the --colors / --highlight deletion is
undone.

Test plan

  • go test ./shortcuts/sheets/... -count=1 green (full suite + new dropdown coverage)
  • Affected dry-run + execute-path assertions updated (TestDimMove_DryRun, TestExecute_DimMove, TestDropdownSet_CellsShape, TestDropdownSet_ColorsLongerThanOptions, TestDropdownSet_ColorsShorterAccepted, TestDropdownGet table, TestWorkbookCreate_DryRun, TestRangeSort_RejectsMalformedKeys, TestBatchOp_BodyMatchesStandalone/+range-sort, TestDropdownUpdate_BatchPayload)
  • go build ./... green
  • Rebased onto latest feat/lark-sheets-refactor; re-synced from spec MR !7
  • PPE smoke (ppe_lark_cli_sheet): replay TC-007 / TC-027030 / TC-046 / TC-038 / R2-003006 / R2-009 / R2-012; plus new dropdown smoke covering --colors / --highlight end-to-end against the upstream highlight_colors / enable_highlight fields, with shorter-than-options cases
  • Bot-identity matrix (TC-006/078~085) — out of scope for this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 67af696d-243b-4138-a1ff-fd36752095f6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths size/M Single-domain feat or fix with limited business impact and removed size/L Large or sensitive change across domains or core paths size/M Single-domain feat or fix with limited business impact labels May 22, 2026
@zhengzhijiej-tech zhengzhijiej-tech changed the title fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema (restore --colors/--highlight) May 22, 2026
@xiongyuanwen-byted xiongyuanwen-byted force-pushed the feat/lark-sheets-refactor branch from c0b1ffa to 370137e Compare May 24, 2026 05:03
@zhengzhijiej-tech zhengzhijiej-tech force-pushed the fix/sheets-e2e-fixes-batch1 branch from 514b5e1 to 538eb2e Compare May 25, 2026 03:25
…ort with server schema

Five separate E2E failures in shortcuts/sheets/ that all trace back to a
CLI ↔ server contract mismatch. Each is independently scoped; bundling
them because they share the test-report citation and the same one-line
fix shape in most cases.

buildInitialFillInput sent {"sheet_id": ""} on the secondary
set_cell_range call after creating the workbook. The empty value was a
holdover from "...otherwise server picks first sheet" — but
set_cell_range rejects an empty selector with
"sheet_id or sheet_name is required" rather than falling back to the
default sheet.

Use sheet_name "Sheet1" instead. POST /sheets/v3/spreadsheets always
creates that sheet on workbook creation, and set_cell_range accepts
sheet_name as an equivalent selector — saves an extra
get_workbook_structure round-trip just to learn the auto-generated id.

buildDropdownValidation emitted four fields that don't exist in the
canonical set_cell_range.data_validation schema:

  - "values" (options list)       → renamed to "items"
  - "multiple_values"              → renamed to "support_multiple_values"
  - "colors" (per-option color)    → removed (not in schema; flag also
                                     removed from data/flag-defs.json
                                     for +dropdown-set / -update)
  - "highlight_options"            → removed (not in schema; flag also
                                     removed)

The canonical schema lives at sheet-skill-spec/canonical-spec/tool-
schemas/mcp-tools.json (set_cell_range tool, data_validation property);
the colors / highlight knobs were CLI inventions the server never
accepted, so removing the flags is correct (renaming would leave the
flags broken). Skill reference docs (write-cells.md, batch-update.md)
synced.

validateDropdownOptionsColors lost its colors check; renamed to
validateDropdownOptions to reflect the narrower contract.

dropdownGetInput sent "Sheet1!C2:C6" verbatim as a ranges[] entry.
get_cell_ranges expects sheet_id / sheet_name as separate fields and
ranges entries without the sheet prefix; the server bounced with
"sheet not found, sheetId:" (empty).

Use the existing splitSheetPrefixedRange helper (declared in
lark_sheet_batch_update.go) to break "Sheet1!C2:C6" into ("Sheet1",
"C2:C6"), then thread the sheet name through sheetSelectorForToolInput
exactly like +cells-get does.

The shortcut was POSTing to /sheets/v2/spreadsheets/{token}/dimension_
range, which is the v2 insert-dimension endpoint and requires a top-
level {"dimension": {...}} body. Move uses a separate endpoint:

  POST /sheets/v2/spreadsheets/{token}/move_dimension
  body: { "source": {...}, "destination_index": N }

(camelCase "destinationIndex" → snake_case "destination_index" to
match the v2 contract.) Both DryRun and Execute updated, plus the
TestDimMove_DryRun and TestExecute_DimMove assertions.

transform_range.sort_conditions[i] requires both `column` (string) and
`ascending` (bool); rangeSortInput passed the --sort-keys array through
to the server unvalidated, so missing fields surfaced as opaque
"required property X missing" errors with no per-item context.

Walk the parsed array client-side, reject with item-pointing messages.
Test fixtures and a contract-test fixture switched from the historical
{col, order} vocabulary (which the server has never accepted) to the
correct {column, ascending}.

Server-schema citations and test-report case mapping in this branch's
plan file.
data/flag-defs.json is regenerated from the upstream sheet-skill-spec
canonical-spec; editing it here gets clobbered on the next sync. The
schema realignment for +dropdown-set / -update --colors / --highlight
removal needs to land on the base table first, then flow back through
sheet-skill-spec → larksuite-cli sync, not via a direct CLI-side edit.

Restore the previous flag entries verbatim. The Go-side change in
buildDropdownValidation still drops the wire fields, so:

  - users passing --colors / --highlight today see the flag accepted
    silently (no effect on the wire) until the upstream removal lands;
  - after upstream removal + sync, both the flag declarations and the
    Go-side handling will be in sync.

Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
These md files are sync targets generated from sheet-skill-spec; editing
them here gets clobbered on the next sync, same as data/flag-defs.json.
The --colors / --highlight row removals belong on the upstream base
table → canonical-spec sync, not here.

Restore the previous --colors / --highlight rows in both
lark-sheets-write-cells.md (+dropdown-set) and lark-sheets-batch-update.md
(+dropdown-update). The Go-side change in buildDropdownValidation still
drops the wire fields, so:

  - users passing --colors / --highlight today see the flag accepted
    silently (no effect on the wire) until upstream removes the flag;
  - after upstream removal + sync, both flag declarations, ref docs, and
    Go-side handling will be in sync.

Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
… --highlight

Upstream sheet-skill-spec base table deleted the --colors and --highlight
flags on +dropdown-set / +dropdown-update (the corresponding wire fields
data_validation.colors / .highlight_options were never accepted by the
server schema; see prior fix in this branch). Re-running the sync from
canonical-spec brings the CLI flag-defs and skill reference docs back in
line with the Go-side handling that already drops these fields.

Generated by `npm run sync:cli` in sheet-skill-spec @ ac7acef.
…al fields

Reverses the --colors / --highlight removal from 7932ab2 (item larksuite#2 of the
batch-1 schema-alignment commit). That commit dropped both flags after the
test report flagged data_validation.colors / highlight_options as "unexpected
property" — at the time the canonical set_cell_range.data_validation schema
listed only help_text / items / operator / range / support_multiple_values /
type / values, so the flags had no server-side target and the removal was
correct.

Since then, set_cell_range.data_validation has gained two fields explicitly
modelling the dropdown highlight UI (mcp-tools.json in sheet-skill-spec
2026-05-22 base sync):

  enable_highlight  (bool)       — show pill backgrounds
  highlight_colors  (string[])   — hex pill colors, length must match items

So the flags are back, but rewired:

  --colors    -> data_validation.highlight_colors    (was: colors)
  --highlight -> data_validation.enable_highlight    (was: highlight_options)

--options -> items and --multiple -> support_multiple_values renames from
7932ab2 are kept.

Changes:

- buildDropdownValidation: re-add --colors / --highlight handling against
  the new field names; --colors length check stays inline (so dropdownSetInput
  Validate path catches it via validateViaInput, no separate guard needed).
- validateDropdownOptions -> validateDropdownOptionsColors: restore the
  Validate-time --colors length check on +dropdown-update / +dropdown-delete
  (called from lark_sheet_batch_update.go).
- TestDropdownSet_CellsShape: extend to assert highlight_colors /
  enable_highlight emitted; assert legacy `colors` / `highlight_options`
  absent.
- TestDropdownSet_ColorsLengthMismatch: new — covers the early Validate
  error path.
- TestDropdownUpdate_BatchPayload: extend to cover dropdownBatchInput
  propagation of --colors / --highlight through batch_update.
- skills/lark-sheets/references/lark-sheets-{write-cells,batch-update}.md,
  shortcuts/sheets/data/flag-defs.json, flag-schemas.json: synced from
  sheet-skill-spec generate output (MR !7).
Catches up to sheet-skill-spec's 2026-05-25 base sync (MR !7) after
rebasing onto upstream feat/lark-sheets-refactor (12 new upstream commits
including the lark-sheets skill refactor + tools-schema migration).

Spec changes flowing in:

- highlight_colors description loosened: length may be **shorter than**
  --options (server cycles remaining slots through a built-in 10-color
  palette); previously the tool errored on any length mismatch.
- shortcuts/sheets/data/flag-schemas.json: mass re-mirror — generator now
  emits `type` before `properties` and adds explicit `additionalProperties:
  false` on object schemas (cosmetic, no behavior change).
- skills/lark-sheets/references/lark-sheets-{batch-update,chart,write-cells}.md:
  --options gains the type='list' tag; data_validation inline field-count
  goes 7 → 9 (catches up the highlight schema in the summary); chart
  position / size marked optional per upstream.

Go-side adjustment:

- buildDropdownValidation / validateDropdownOptionsColors: change the
  --colors length check from strict-equal to "must not exceed --options"
  to match the relaxed schema.
- TestDropdownSet_ColorsLengthMismatch -> TestDropdownSet_ColorsLongerThanOptions
  (now hits the overflow path with 3 colors vs 2 options).
- New TestDropdownSet_ColorsShorterAccepted: 2 colors vs 4 options is
  legal and forwarded as-is.
Mirrors sheet-skill-spec MR !7 changes:

- skills/lark-sheets/references/lark-sheets-write-cells.md: new "Dropdown
  配色" section explaining how --colors (→ data_validation.highlight_colors)
  and --highlight (→ data_validation.enable_highlight) compose — length
  rule (shorter ok, longer rejected), --highlight gating, palette
  fallback behavior, minimal +dropdown-set example.
- skills/lark-sheets/references/lark-sheets-batch-update.md: one-line
  pointer to the write_cells section for +dropdown-update / -delete
  (same rules).
- shortcuts/sheets/data/flag-defs.json: --colors / --highlight `desc`
  fields gain the long-form server-field / length-rule descriptions
  used by `--help`.

No Go-side change — earlier commit 538eb2e already loosened the
buildDropdownValidation length check to "must not exceed"; this PR step
just makes the docs / `--help` text catch up.
…mode

Previously +dropdown-set / +dropdown-update only emitted
data_validation.type=list — agents wanting listFromRange (dropdown options
sourced from existing cells, kept in sync with that range) had to drop down
to +cells-set and hand-build a data_validation map. The flag now exposes it
natively as --source-range, paired with --options under XOR.

CLI changes:

- shortcuts/sheets/lark_sheet_write_cells.go:
  * new dropdownTypeAndItems(runtime) — central XOR resolver: rejects 0 or
    2 of {--options, --source-range}, returns (sourceSize, partial dv with
    type+items|range filled in). Source size = options length for list
    mode, rangeDimensions(--source-range) cell count for listFromRange.
  * buildDropdownValidation rewritten to call the resolver, then layer
    --colors / --multiple / --highlight on top — semantics unchanged
    for callers, just two modes instead of one.
  * validateDropdownOptions / -Colors renamed to validateDropdownSourceOrOptions
    so the XOR + length check fires at +dropdown-update Validate time too.
  * --colors length error message generalized: "must not exceed dropdown
    source size (N)" (covers both modes).
- shortcuts/sheets/lark_sheet_batch_update.go: rename call site.
- shortcuts/sheets/lark_sheet_write_cells_test.go: 4 new tests —
  ListFromRange (happy path: range + items absent + colors + highlight all
  emit), ListFromRange_ColorsLongerThanCells (overflow against T1:T3 cell
  count), XorBothSet, XorNeitherSet. Updated the existing
  ColorsLongerThanOptions assertion to match the new "source size" wording.

Spec-driven changes (synced via npm run sync:cli from sheet-skill-spec
MR !7 2c298b6):

- shortcuts/sheets/data/flag-defs.json: --options Required flips to xor on
  +dropdown-set/-update; new --source-range row gains long-form description
  pointing at server data_validation.range + the XOR semantics.
- skills/lark-sheets/references/lark-sheets-write-cells.md: "Dropdown 配色"
  section reorganized into "Dropdown 选项 + 配色" — XOR comparison table
  (list vs listFromRange), shared config flag table (--highlight /
  --colors), explicit length rule covering both modes, side-by-side
  minimal examples, server-range-normalization gotcha callout.
- skills/lark-sheets/references/lark-sheets-batch-update.md pointer updated
  to mention both modes + that +dropdown-delete is unaffected.

PPE smoke (ppe_lark_cli_sheet) on UFJxszjrZhZ1LVtc9FdcICSbn6b C column:
- +cells-set C1 → "性别" (bold + centered): updated_cells_count=1
- +dropdown-set --range C2:C21 --source-range "Sheet1!T1:T3" --colors
  '["#cce8ff","#ffd6e7","#e6e6e6"]' --highlight: updated_cells_count=20
- read-back: data_validation.type=listFromRange + range=$T$1:$T$3 (server
  normalizes the prefix away on storage; highlight_colors /
  enable_highlight not echoed by get_cell_ranges, see byted-sheet read
  projection TODO).
- error-path replay (both XOR violations + colors > source-size) all
  rejected at Validate stage with the expected messages.
Mirrors sheet-skill-spec MR !7 60df610 — narrative now describes how the
flags interact (XOR, colors length rule, highlight gating, sheet-prefix
read-back gotcha) without exposing the underlying data_validation field
names or server-side normalization details that agents don't act on.

No Go-side change, no shortcut behavior change.
The earlier commit 49104ec swapped --colors out of parseJSONFlag's "Used
by" example list when it deleted the flag (item larksuite#2 there removed --colors
/ --highlight from +dropdown-set/-update). Subsequent commits 8672d8e /
538eb2e / fb90c8b reinstated --colors (and added --source-range) but did
not roll back this docstring tweak — leaving an orphan reference to
--properties where --colors used to be.

This restores the example list to its pre-49104ec form so the docstring
matches what the helper actually services on this branch's HEAD.

Pure docstring change — function behavior unaffected, no test movement.
…rksuite#1

Two test fallouts from rebasing onto upstream 4be06c8 (which independently
re-fixed +workbook-create and +dim-move with a more thorough approach):

- shortcuts/sheets/lark_sheet_workbook_test.go: our PR's earlier
  TestWorkbookCreate_DryRun "with headers and data → 2-step plan" subtest
  asserted the expedient sheet_name="Sheet1" / no-sheet_id wire body that
  matched our dropped fix larksuite#1 implementation. Upstream's fix larksuite#1 resolves
  the workbook's first sheet via get_workbook_structure and fills with
  the real sheet_id instead. Reset this file to upstream's version — our
  superseded assertions disappear, upstream's tests cover the new wire
  shape.

- shortcuts/sheets/execute_paths_test.go: TestExecute_RangeSort fixture
  still used the legacy {col, order} sort-key shape because the rebase
  resolution picked the upstream version of this file wholesale (it
  contained other unrelated changes). Re-apply just the one fixture
  update to {column, ascending} so fix larksuite#5's CLI-side rejection logic
  exercises a valid input — server-side sort_conditions has required
  fields `column` (string) and `ascending` (bool); the historical
  {col, order} vocabulary was never accepted.

go build ./... + go test ./shortcuts/sheets/... -count=1 both green.
@zhengzhijiej-tech zhengzhijiej-tech force-pushed the fix/sheets-e2e-fixes-batch1 branch from 35bf25e to f0dea38 Compare May 25, 2026 06:42
@zhengzhijiej-tech zhengzhijiej-tech merged commit 5eaa70b into larksuite:feat/lark-sheets-refactor May 25, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant